Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Increase max-height on button in welcome email template #20447

Merged
merged 2 commits into from
Apr 17, 2020
Merged

Increase max-height on button in welcome email template #20447

merged 2 commits into from
Apr 17, 2020

Conversation

liamjack
Copy link
Contributor

Fixes #13287 implementing @fuanegua's fix proposal (#13287 (comment)) by increasing the max-height of the button to 60px from 40px.

Before:

Short Nextcloud instance name

before_short

Long Nextcloud instance name

before_long

After

Short Nextcloud instance name

after_short

Long Nextcloud instance name

after_long

@liamjack
Fix nextcloud#13287 - Increase max-height on button in welcome email …
301335e 
…template

Signed-off-by: Liam JACK <liamjack@users.noreply.github.com>
@gary-kim gary-kim added 3. to review Waiting for reviews bug design Design, UI, UX, etc. labels Apr 12, 2020
@gary-kim gary-kim added this to the Nextcloud 19 milestone Apr 12, 2020
@kesselb
Copy link
Contributor

kesselb commented Apr 12, 2020 •
edited
Loading

Would it be possible to have the same size for the install client button?

Thanks for attaching screenshots. That makes reviewing much easier 👍

@liamjack
Copy link
Contributor Author

I've had a go, but I'm unable to make the "install client" button have the same height as the "go to" button, sorry !

@rullzer rullzer mentioned this pull request Apr 13, 2020
Merged
59 tasks
jancborchardt
jancborchardt approved these changes Apr 14, 2020
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks a lot for your contribution @liamjack! :)

@jancborchardt
Copy link
Member

@liamjack @kesselb @fuanegua could you also review Fix design and layout of notification mails #20380 which is very related? Thanks!

This was referenced Apr 15, 2020
Merged
Merged
@MorrisJobke MorrisJobke changed the title Fix #13287 - Increase max-height on button in welcome email template Increase max-height on button in welcome email template Apr 17, 2020
@MorrisJobke
Fix tests
840bed9 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke
Copy link
Member

I pushed a fix to the two missing test cases. This should be fine then.

MorrisJobke
MorrisJobke approved these changes Apr 17, 2020
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works 👍

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 17, 2020
@skjnldsv skjnldsv merged commit 36f9ad3 into nextcloud:master Apr 17, 2020
@welcome
Copy link

welcome bot commented Apr 17, 2020

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@skjnldsv
Copy link
Member

AH, I might have been a little too fast

2) OCA\Settings\Tests\Mailer\NewUserMailHelperTest::testGenerateTemplateWithoutPasswordResetToken
391 | Failed asserting that two strings are equal.
392 | --- Expected
393 | +++ Actual
394 | @@ @@
395 | Install Client: https://nextcloud.com/install/#install-clients\n
396 | \n
397 | \n
398 | ---\n
399 | -TestCloud -\n
400 | +-- \n
401 | +TestCloud - \n
402 | This is an automatically sent email, please do not reply.'
403 |  
404 | /drone/src/apps/settings/tests/Mailer/NewUserMailHelperTest.php:624
405 |  
406 | 3) OCA\Settings\Tests\Mailer\NewUserMailHelperTest::testGenerateTemplateWithoutUserId
407 | Failed asserting that two strings are equal.
408 | --- Expected
409 | +++ Actual
410 | @@ @@
411 | Install Client: https://nextcloud.com/install/#install-clients\n
412 | \n
413 | \n
414 | ---\n
415 | -TestCloud -\n
416 | +-- \n
417 | +TestCloud - \n
418 | This is an automatically sent email, please do not reply.'
419 |  
420 | /drone/src/apps/settings/tests/Mailer/NewUserMailHelperTest.php:848
421 |  

Let's see tests

@rullzer
Copy link
Member

rullzer commented Apr 17, 2020

Fix for the tests in #20540

@liamjack liamjack deleted the fix/13287/welcome_email branch April 18, 2020 09:09
@rullzer
Copy link
Member

rullzer commented Jun 12, 2020

/backport to stable18

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jun 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@blizzz blizzz Awaiting requested review from blizzz

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@kesselb kesselb Awaiting requested review from kesselb

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish bug design Design, UI, UX, etc. feature: emails
Projects
None yet
Milestone
Nextcloud 19
Development

Successfully merging this pull request may close these issues.

"Set your password" text on button in welcome mail out of bounds
7 participants
@liamjack @kesselb @jancborchardt @MorrisJobke @skjnldsv @rullzer @gary-kim